-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update app-model
doc with new information about _mock_app
fixture
#403
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
of every test. | ||
Since the `_mock_app` fixture is used in `make_napari_viewer`, plugins and other | ||
external users that write tests using `make_napari_viewer` will also have the benefit | ||
of a new app for each test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we mention that it is exported just like make_napari_viewer
so envs with napari
installed will be able to use these fixtures? We do mention this when we talk about make_napari_viewer
: https://napari.org/dev/developers/contributing/testing.html#make-napari-viewer
(ignore the failed include, I think thats fixed in #393)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about it but since we've kept it private (kinda with the underscore), I'd rather not mention it explicitly as I don't necessarily want to encourage direct usage of it by external packages. If we find use for it I'd be happy to amend then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, good point.
Slightly related. could we add a link to the make_napari_viewer
, maybe further up, when we first mention it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you!
References and relevant issues
Depends on napari/#6823
Description
Updates the information about
_mock_app
to state that we use it explicitly in_make_napari_viewer
and allother tests that require a
NapariApplication
.Also make note that plugins that use
make_napari_viewer
now get the benefit of the_mock_app
.